-
-
Notifications
You must be signed in to change notification settings - Fork 108
Fix: Gracefully handle syntax errors in definePage to prevent Vite server crashes #692
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Deploying unplugin-vue-router with
|
| Latest commit: |
f41937a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0b37225b.unplugin-vue-router.pages.dev |
| Branch Preview URL: | https://copilot-fix-904c72ad-c6f9-42.unplugin-vue-router.pages.dev |
src/core/definePage.spec.ts
Outdated
|
|
||
| describe('error handling', () => { | ||
| it('handles syntax errors gracefully when extracting definePage', async () => { | ||
| const codeWithSyntaxError = ` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this variable for a piece of code can be extracted in the outer scope. Share it among tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extracted the codeWithSyntaxError variable to the outer scope of the error handling describe block to share it among the tests, avoiding duplication. (5992675)
src/core/definePage.spec.ts
Outdated
| expect(result).toBeUndefined() | ||
| }) | ||
|
|
||
| it('handles malformed definePage object gracefully', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is redundant, syntax errors are already tested. Ensure you don't have duplicated testss
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the redundant "handles malformed definePage object gracefully" test as it was duplicating syntax error testing already covered by other tests. (5992675)
src/core/definePage.spec.ts
Outdated
| expect(result).toBe('export default {}') | ||
| }) | ||
|
|
||
| it('handles completely invalid JavaScript syntax gracefully', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here, remove it, it's just syntax error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the redundant "handles completely invalid JavaScript syntax gracefully" test as it was just another syntax error variation already covered by existing tests. (5992675)
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Comment |
commit: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat/experimental-router #692 +/- ##
===========================================================
Coverage ? 61.10%
===========================================================
Files ? 36
Lines ? 3409
Branches ? 626
===========================================================
Hits ? 2083
Misses ? 1319
Partials ? 7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@copilot address the review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dc3f8ef to
e5a73fc
Compare
e824db5 to
8366aae
Compare
Co-authored-by: posva <[email protected]>
…, remove redundant tests Co-authored-by: posva <[email protected]>
…fix syntax error warning handling Co-authored-by: posva <[email protected]>
f41937a to
c623413
Compare
Problem
Syntax errors in
definePage()calls would crash the Vite development server, making it impossible to continue development when there were JavaScript syntax issues in the page definition. This created a poor developer experience where a simple typo could bring down the entire dev server.Solution
Enhanced error handling throughout the
definePageprocessing pipeline to gracefully handle syntax errors and parsing failures:Key Changes
Added comprehensive try-catch blocks in
definePageTransform()andextractDefinePageNameAndPath()functions to catch any parsing or processing errorsEnhanced
getCodeAst()function with error handling around Babel AST parsing to prevent syntax errors from propagatingChanged error behavior from throwing
SyntaxErrors to logging warnings and returning safe fallback values:'export default {}'undefined(no transform)Improved developer feedback by replacing crashes with helpful warning messages that identify the problematic file
Testing
Added comprehensive test coverage for various syntax error scenarios:
Impact
The fix ensures a more robust development experience while maintaining all existing functionality for properly formatted
definePage()calls.✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.